Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(application): commands, pre-run validation #5007

Merged
merged 4 commits into from
Sep 4, 2024
Merged

refactor(application): commands, pre-run validation #5007

merged 4 commits into from
Sep 4, 2024

Conversation

mr-cal
Copy link
Collaborator

@mr-cal mr-cal commented Aug 29, 2024 •
edited
Loading

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox run -m lint?
  • Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)

Refactor

  1. Use craft-cli command groups
  2. Stop overriding craft-applications command_groups property, except for the LifecycleGroup
  3. Split the logic in _get_dispatcher() code into 2 places:
    • Check and raise ClassicFallback errors before creating the dispatcher
    • Check and raise CraftErrors if the wrong codepath or wrong version of snapcraft was chosen after creating the dispatcher

UX changes

  1. ESM errors show a properly formatted error instead of a traceback
  2. Remote build errors about SNAPCRAFT_REMOTE_BUILD_STRATEGY show a properly formatted error instead of a traceback
  3. SNAPCRAFT_REMOTE_BUILD_STRATEGY is only validated when executing snapcraft remote-build
  4. SNAPCRAFT_REMOTE_BUILD_STRATEGY is validated for core24 and newer snaps, rather than being ignored.
  5. snapcraft list-plugins --base=core20 no longer fails in a core24 project directory (snapcraft list-plugins --base=core20 fails in a core24 project directory #5008)

Fixes #4911
Fixes #5008
(CRAFT-3106)

@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from 2c57004 to a4ca44f Compare August 29, 2024 13:46
@mr-cal mr-cal marked this pull request as draft August 29, 2024 14:31
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from a4ca44f to 5ab9475 Compare August 29, 2024 15:32
mr-cal
mr-cal commented Aug 29, 2024
View reviewed changes
snapcraft/application.py Outdated Show resolved Hide resolved
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch 3 times, most recently from 6279969 to 3cb2e1f Compare August 30, 2024 14:02
@mr-cal mr-cal marked this pull request as ready for review August 30, 2024 15:40
@mr-cal mr-cal requested review from tigarmo and cmatsuoka August 30, 2024 16:13
@mr-cal mr-cal added the rebase label Aug 30, 2024
tigarmo
tigarmo requested changes Sep 2, 2024
View reviewed changes
snapcraft/application.py Outdated Show resolved Hide resolved
snapcraft/application.py Show resolved Hide resolved
@mr-cal mr-cal requested a review from tigarmo September 3, 2024 16:22
@mr-cal
refactor: use craft-cli command groups
fb38557 
CommandGroups can be used now that craft-application will accept a
sequence of commands.
See canonical/craft-application#359

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal
style: reformat dispatcher error messages
055b0fe
@mr-cal
refactor(application): redesign pre-run validation
2aef667 
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal
refactor: enable craft-parts features in the correct location
4403441 
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from 48d8d63 to 4403441 Compare September 4, 2024 19:39
cmatsuoka
cmatsuoka approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, thanks!

@mr-cal mr-cal merged commit 3508cf3 into main Sep 4, 2024
8 of 10 checks passed
@mr-cal mr-cal deleted the work/CRAFT-3106/command-refactor branch September 4, 2024 21:03
@mr-cal mr-cal mentioned this pull request Nov 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tigarmo tigarmo tigarmo approved these changes

@cmatsuoka cmatsuoka cmatsuoka approved these changes

Assignees
No one assigned
Labels
rebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

snapcraft list-plugins --base=core20 fails in a core24 project directory Refactor command_group usage
3 participants
@mr-cal @cmatsuoka @tigarmo